Skip to content

feat(node): blind recipient identities at rest and gate B1 by repo readability#40

Open
beardthelion wants to merge 3 commits into
Gitlawb:mainfrom
beardthelion:feat/recipient-blinding-at-rest
Open

feat(node): blind recipient identities at rest and gate B1 by repo readability#40
beardthelion wants to merge 3 commits into
Gitlawb:mainfrom
beardthelion:feat/recipient-blinding-at-rest

Conversation

@beardthelion

@beardthelion beardthelion commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Completes recipient-set blinding for encrypted replication: the reader set of a withheld blob is no longer stored or published anywhere in cleartext, only in the encrypted envelope key-wraps.

This stacks on #36 (encrypted replication B1/B2/B3, which already blinds the envelope, the peer-replication wire, and the Arweave manifest). Since the upstream repo only has main, the diff here carries that stack as prerequisites; the new work in this PR is the at-rest change.

What changed

  • The encrypted_blobs.recipients column (a cleartext DID list) is replaced (migration v5) with an opaque, node-keyed recipients_tag (HMAC-SHA256 over the sorted DID set). The tag is used only to detect a recipient-set change so an unchanged blob is not re-sealed; it is never the DID list.
  • B1 discovery (/encrypted-blobs) and fetch (/encrypted-blob/{oid}) are now authorized by repo readability (the same visibility_check the git read path uses) rather than per-recipient matching. Decryption is gated by the envelope crypto: only a real recipient can open an envelope.
  • encrypt_and_pin keys the tag from the node seed and returns {oid, cid}; the Arweave manifest tuple drops the now-unused recipient vec; the mirror path stores no recipient data.

Why

An origin that enforces per-recipient authorization must be able to test whether a caller is a recipient, which means it can always recover the reader set. Moving authorization onto repo readability plus envelope crypto takes the origin out of that loop, so a DB compromise no longer reveals who can read a blob.

Accepted trade-offs

  • A repo-reader who is not a recipient of a given blob can see that the blob exists ({oid, cid}). The OID is already in the tree they clone and the envelope bytes are already public on IPFS, so no new secret is exposed.
  • A full origin compromise (DB plus the node key) could still brute-force the reader set from the keyed tag. The tag is only a re-seal change-detector; authorization no longer depends on it.

Tests

recipients_tag is covered by unit tests (order-insensitive, set-sensitive, node-keyed). The handler authorization change is wiring around the existing, tested visibility_check.

Summary by CodeRabbit

  • Improvements
    • Enhanced access control for encrypted blobs through repository visibility verification
    • Optimized encrypted data replication and synchronization workflows
    • Improved privacy by reducing storage of recipient identity information in manifests and metadata

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 43dd6dbc-3348-487b-b77b-771f6cda642b

📥 Commits

Reviewing files that changed from the base of the PR and between d0a059b and 6b7bf15.

📒 Files selected for processing (7)
  • .gitignore
  • crates/gitlawb-node/src/api/encrypted.rs
  • crates/gitlawb-node/src/api/repos.rs
  • crates/gitlawb-node/src/arweave.rs
  • crates/gitlawb-node/src/db/mod.rs
  • crates/gitlawb-node/src/encrypted_pin.rs
  • crates/gitlawb-node/src/sync.rs
💤 Files with no reviewable changes (1)
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (4)
  • crates/gitlawb-node/src/api/encrypted.rs
  • crates/gitlawb-node/src/sync.rs
  • crates/gitlawb-node/src/encrypted_pin.rs
  • crates/gitlawb-node/src/arweave.rs

📝 Walkthrough

Walkthrough

Removes cleartext recipient DID storage from the encrypted blob pipeline. A DB migration (v5) drops the recipients column and adds a recipients_tag TEXT column; encrypt_and_pin now accepts node_seed and computes an HMAC-SHA256 fingerprint to skip resealing when the recipient set is unchanged. API handlers, Arweave manifest anchoring, and mirror sync replication are updated to work with (oid, cid) pairs only.

Changes

Recipient identity removal from encrypted blob pipeline

Layer / File(s) Summary
DB schema migration and encrypted blob accessors
crates/gitlawb-node/src/db/mod.rs
Migration v5 drops the cleartext recipients column and adds recipients_tag TEXT DEFAULT ''; record_encrypted_blob now upserts cid and recipients_tag; list_all_encrypted_blobs returns only (oid, cid); encrypted_blob_cid drops the caller parameter; new encrypted_blob_recipients_tag accessor is added.
Encrypt-then-pin with recipients_tag and node_seed
crates/gitlawb-node/src/encrypted_pin.rs, crates/gitlawb-node/src/api/repos.rs
Adds recipients_tag (node-seeded HMAC-SHA256 fingerprint over recipient DID set); encrypt_and_pin accepts node_seed, computes per-blob tag, queries DB to skip resealing when tag matches, and records (oid, cid, recipients_tag); git_receive_pack captures node_seed and passes it to the call.
Arweave manifest blob shape
crates/gitlawb-node/src/arweave.rs
EncryptedManifest::blobs changes from (oid, cid, recipients) to (oid, cid) tuples; adds empty-blob early-return no-op; serialization and all unit tests updated to assert recipient fields are absent.
Encrypted blob API handlers
crates/gitlawb-node/src/api/encrypted.rs
list_encrypted_blobs, get_encrypted_blob, and replicate_encrypted_blobs all gain visibility_check access gating; DB calls drop caller/recipient arguments; get_encrypted_blob fetches envelope bytes via IPFS cat.
Mirror sync and .gitignore cleanup
crates/gitlawb-node/src/sync.rs, .gitignore
replicate_encrypted_blobs collects (oid, cid) directly from the updated DB method and passes "" for recipients_tag when recording replicated blobs; removes the docs/superpowers/ ignore rule.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • Gitlawb/node#36: Direct overlap — this PR updates the same api/encrypted.rs, db/mod.rs, encrypted_pin.rs, and arweave.rs files introduced or modified in #36's encrypted replication work, removing stored recipient identities from those paths.
  • Gitlawb/node#25: The visibility_check gating added to all three encrypted blob API handlers builds directly on the path-scoped visibility rules API introduced in this PR.

Suggested reviewers

  • kevincodex1

Poem

🐇 Hop, hop, the secrets stay sealed,
No DID lists in the DB revealed!
A HMAC tag, node-keyed and bright,
Checks if recipients changed overnight.
The blobs are pinned, the manifest clean—
Just oid and cid, no identities seen! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: blinding recipient identities at rest and gating B1 (discovery/fetch endpoints) by repo readability.
Description check ✅ Passed The description covers the main changes (migration v5, authorization shift, encrypt_and_pin updates), motivation (preventing origin from needing to validate recipients), trade-offs, and testing. All key template sections are addressed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/gitlawb-node/src/api/repos.rs (1)

327-340: ⚠️ Potential issue | 🔴 Critical

Gate service=git-receive-pack info/refs with the same visibility/RepoNotFound behavior as git-upload-pack

  • crates/gitlawb-node/src/server.rs mounts GET /{owner}/{repo}/info/refs with auth::optional_signature, so unauthenticated callers can hit this handler.
  • In crates/gitlawb-node/src/api/repos.rs (git_info_refs), the visibility_check(...)->Decision::Deny path that maps to AppError::RepoNotFound(...) runs only for service == "git-upload-pack".
  • For service == "git-receive-pack", the handler skips visibility_check, still acquires the repo (acquire_fresh) and returns smart_http::info_refs(...), which can leak private-repo existence and advertised refs.

Add the same read-visibility gate for service == "git-receive-pack" and keep mapping deny/unauthorized to RepoNotFound to avoid an existence oracle and ref disclosure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/api/repos.rs` around lines 327 - 340, The handler
currently only enforces read visibility for service == "git-upload-pack"; update
the branch handling of service == "git-receive-pack" in git_info_refs (where
service, record, auth, and name are in scope) to call
state.db.list_visibility_rules(&record.id).await?, compute caller the same way
(auth.as_ref().map(|e| e.0 .0.as_str())), and run visibility_check(&rules,
record.is_public, &record.owner_did, caller, "/") and if it returns
Decision::Deny return Err(AppError::RepoNotFound(format!("{owner}/{name}"))),
mirroring the existing git-upload-pack logic so unauthenticated/unauthorized
callers do not observe private repo existence or refs.

Source: Learnings

🧹 Nitpick comments (1)
crates/gitlawb-node/src/sync.rs (1)

310-324: 💤 Low value

Pinned envelope with mismatched CID is not unpinned.

When pin_git_object returns a CID that differs from the expected one, the code logs a warning and skips recording, but the incorrectly-pinned content remains in IPFS storage. This is a minor storage leak rather than a correctness issue (the DB row is not created, so the next sync will re-attempt), but over many syncs with a misbehaving IPFS node it could accumulate garbage.

Consider unpinning the mismatched CID before continuing, or document this as an accepted trade-off.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/sync.rs` around lines 310 - 324, The mismatched CID
path currently logs and continues but leaves the wrong content pinned in IPFS;
update the match arm handling Ok(cid) if cid != blob.cid to unpin the unexpected
CID before continuing (call the appropriate unpin helper in crate::ipfs_pin,
e.g., crate::ipfs_pin::unpin_git_object(ipfs_api, &cid) or use ipfs_api
directly), then log any unpin error and continue; keep the existing call to
db.record_encrypted_blob only in the successful CID-equals branch and ensure
references to pin_git_object, blob.oid, blob.cid, db.record_encrypted_blob,
ipfs_api, and envelope are used to locate where to add the unpin/cleanup call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/gitlawb-core/src/identity.rs`:
- Around line 55-59: The method seed_bytes() exposes raw 32-byte private seed as
[u8; 32]; change it to follow the existing to_seed() safety pattern by returning
a zeroizing/secret wrapper instead of a plain array (e.g., use the same type
returned by to_seed() or Zeroizing<[u8;32]>), and update call sites that use
seed_bytes() (e.g., where x25519_secret_from_seed and XSecret::from are invoked)
to accept the secret wrapper or to extract the inner bytes temporarily in a
zeroized scope; keep signing_key and to_seed() as the canonical source of seed
material and remove plain [u8;32] returns to ensure automatic zeroization.

In `@crates/gitlawb-node/src/api/encrypted.rs`:
- Around line 40-42: Update the endpoint doc comment for GET
/api/v1/repos/{owner}/{repo}/encrypted-blob/{oid} (function get_encrypted_blob)
to reflect current authorization: state that the handler returns raw envelope
bytes when the repository is readable by the caller (repo read permission),
rather than claiming recipient-based access; keep any note about additional
envelope-level checks if still applicable but remove the outdated "recipient"
wording so clients and future maintainers see the correct contract.
- Around line 75-90: The replicate_encrypted_blobs handler currently calls
state.db.list_all_encrypted_blobs(&record.id) without verifying read access;
before listing blobs, perform the repo-readability gate using the app's
authorization helper (e.g. call a method like
state.ensure_repo_readable(&record) or state.auth.ensure_repo_readable(&owner,
&repo, &record.id)), return an authorization error if the caller cannot read the
repo, and only then call state.db.list_all_encrypted_blobs(&record.id); ensure
the check happens in replicate_encrypted_blobs immediately after get_repo(...)
and before constructing blobs so private repo metadata is never exposed to
unauthorized callers.

In `@crates/gitlawb-node/src/encrypted_pin.rs`:
- Around line 71-74: The loop silently skips when
crate::git::store::read_object(repo_path, oid) returns Err or Ok(None); change
those silent continues to log the failure before continuing: capture the Err
detail and log it (including repo_path and oid) and also log a warning when
Ok(None) (missing blob) so missing encrypted blobs are diagnosable; apply the
same change to the similar branch later (the second read_object check at lines
~82-85) so both read_object failure paths emit contextual logs rather than
silently continuing.
- Around line 61-65: The current code treats errors from
db.encrypted_blob_recipients_tag(repo_id, oid).await as cache-misses and
proceeds to reseal/pin, causing external writes during DB failures; change the
logic to match on the Result explicitly: handle Ok(Some(stored_tag)) -> compare
to tag and continue if equal, Ok(None) -> proceed with reseal, and Err(e) ->
propagate or return the error (do not continue to reseal/pin). Use
encrypted_blob_recipients_tag, repo_id, oid and tag in the match and propagate
the DB error via the function's error return (or use the ? operator) instead of
swallowing it.

In `@crates/gitlawb-node/src/git/visibility_pack.rs`:
- Around line 21-24: The current loop in visibility_pack.rs (the refname check
inside the for (refname, _oid) in refs) only keeps refs under refs/heads/* and
refs/tags/*, which mismatches the broader object enumeration used elsewhere
(smart_http.rs uses rev-list --objects --all and ipfs_pin.rs filters all repo
objects), so update visibility_pack.rs to use the same ref/object enumerator as
the pack/pin paths: remove the restrictive
starts_with("refs/heads/")/starts_with("refs/tags/") checks and instead iterate
the full ref namespace (e.g., all refs under refs/* or use the same
rev-list-based enumeration method), so the withheld set includes any object
reachable from all refs just like blob_paths, smart_http.rs, and ipfs_pin.rs do.
Ensure you reference the same mechanism (function or iterator used for rev-list
traversal) so both packing/pinning and withholding share identical ref/object
enumeration.
- Around line 30-31: The code currently continues on a non-zero `listing.status`
which silently skips a failing `git ls-tree` and can leak data; in the function
in visibility_pack.rs where `listing` is handled (the block checking `if
!listing.status.success()`), replace the `continue` with an error return that
fails closed (e.g., return an Err with context indicating the ref and the
`listing.status` and `listing.stderr`/output). Ensure you propagate a failure up
so callers abort fetch/replication instead of treating missing blobs as public.

In `@crates/gitlawb-node/src/ipfs_pin.rs`:
- Around line 75-86: The cat function currently issues
reqwest::Client::new().post(...).send().await? with no timeout; update cat to
use a bounded request timeout (e.g., via
reqwest::Client::builder().timeout(Duration::from_secs(<N>)).build() and then
.post(...).send().await?) or wrap the send/bytes await in
tokio::time::timeout(Duration::from_secs(<N>), ...).await and convert timeout
errors to anyhow errors; reference the cat function and ipfs_api/cid parameters
when making the change and pick a configurable constant or env-driven value for
the timeout so production worst-case envelope/read latency can be tuned.

---

Outside diff comments:
In `@crates/gitlawb-node/src/api/repos.rs`:
- Around line 327-340: The handler currently only enforces read visibility for
service == "git-upload-pack"; update the branch handling of service ==
"git-receive-pack" in git_info_refs (where service, record, auth, and name are
in scope) to call state.db.list_visibility_rules(&record.id).await?, compute
caller the same way (auth.as_ref().map(|e| e.0 .0.as_str())), and run
visibility_check(&rules, record.is_public, &record.owner_did, caller, "/") and
if it returns Decision::Deny return
Err(AppError::RepoNotFound(format!("{owner}/{name}"))), mirroring the existing
git-upload-pack logic so unauthenticated/unauthorized callers do not observe
private repo existence or refs.

---

Nitpick comments:
In `@crates/gitlawb-node/src/sync.rs`:
- Around line 310-324: The mismatched CID path currently logs and continues but
leaves the wrong content pinned in IPFS; update the match arm handling Ok(cid)
if cid != blob.cid to unpin the unexpected CID before continuing (call the
appropriate unpin helper in crate::ipfs_pin, e.g.,
crate::ipfs_pin::unpin_git_object(ipfs_api, &cid) or use ipfs_api directly),
then log any unpin error and continue; keep the existing call to
db.record_encrypted_blob only in the successful CID-equals branch and ensure
references to pin_git_object, blob.oid, blob.cid, db.record_encrypted_blob,
ipfs_api, and envelope are used to locate where to add the unpin/cleanup call.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 774f892c-6ba4-481c-acdb-4008c3174443

📥 Commits

Reviewing files that changed from the base of the PR and between a2217bf and e715c57.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (24)
  • .gitignore
  • crates/gitlawb-core/Cargo.toml
  • crates/gitlawb-core/src/encrypt.rs
  • crates/gitlawb-core/src/identity.rs
  • crates/gitlawb-core/src/lib.rs
  • crates/gitlawb-node/Cargo.toml
  • crates/gitlawb-node/src/api/encrypted.rs
  • crates/gitlawb-node/src/api/mod.rs
  • crates/gitlawb-node/src/api/repos.rs
  • crates/gitlawb-node/src/api/visibility.rs
  • crates/gitlawb-node/src/arweave.rs
  • crates/gitlawb-node/src/db/mod.rs
  • crates/gitlawb-node/src/encrypted_pin.rs
  • crates/gitlawb-node/src/git/mod.rs
  • crates/gitlawb-node/src/git/smart_http.rs
  • crates/gitlawb-node/src/git/visibility_pack.rs
  • crates/gitlawb-node/src/ipfs_pin.rs
  • crates/gitlawb-node/src/main.rs
  • crates/gitlawb-node/src/pinata.rs
  • crates/gitlawb-node/src/server.rs
  • crates/gitlawb-node/src/sync.rs
  • crates/gitlawb-node/src/visibility.rs
  • crates/gl/src/clone.rs
  • crates/gl/src/main.rs

Comment on lines +55 to +59
/// The raw 32-byte Ed25519 seed. Used to derive the X25519 secret for
/// envelope decryption (see `crate::encrypt`).
pub fn seed_bytes(&self) -> [u8; 32] {
self.signing_key.to_bytes()
}

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid exposing raw seed bytes as a plain return type.

Line 57 returns [u8; 32], which duplicates private key material without zeroization and bypasses the existing to_seed() safety pattern.

Suggested fix
-    pub fn seed_bytes(&self) -> [u8; 32] {
-        self.signing_key.to_bytes()
-    }
+    pub(crate) fn seed_bytes(&self) -> Zeroizing<[u8; 32]> {
+        self.to_seed()
+    }
// crates/gitlawb-core/src/encrypt.rs (call site)
let seed = keypair.seed_bytes();
let my_x = XSecret::from(x25519_secret_from_seed(&seed));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-core/src/identity.rs` around lines 55 - 59, The method
seed_bytes() exposes raw 32-byte private seed as [u8; 32]; change it to follow
the existing to_seed() safety pattern by returning a zeroizing/secret wrapper
instead of a plain array (e.g., use the same type returned by to_seed() or
Zeroizing<[u8;32]>), and update call sites that use seed_bytes() (e.g., where
x25519_secret_from_seed and XSecret::from are invoked) to accept the secret
wrapper or to extract the inner bytes temporarily in a zeroized scope; keep
signing_key and to_seed() as the canonical source of seed material and remove
plain [u8;32] returns to ensure automatic zeroization.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked as #41. This is a pre-existing API (the PR only adds a caller); routing seed access through the existing to_seed() zeroizing wrapper touches other call sites, so handling it separately to keep this PR scoped to recipient-set blinding.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@beardthelion — understood, thanks for the context and for opening #41. The scope boundary makes sense; the fix is straightforward once that issue is picked up. I'll leave the concern open here as a reference.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked in #41 to keep this PR scoped to the blinding change. Will route seed access through the zeroizing wrapper there.

Comment thread crates/gitlawb-node/src/api/encrypted.rs
Comment thread crates/gitlawb-node/src/api/encrypted.rs
Comment thread crates/gitlawb-node/src/encrypted_pin.rs Outdated
Comment thread crates/gitlawb-node/src/encrypted_pin.rs
Comment thread crates/gitlawb-node/src/git/visibility_pack.rs
Comment thread crates/gitlawb-node/src/git/visibility_pack.rs
Comment on lines +75 to +86
/// Fetch raw bytes for a CID from the local Kubo node (`/api/v0/cat`).
pub async fn cat(ipfs_api: &str, cid: &str) -> Result<Vec<u8>> {
if ipfs_api.is_empty() {
return Err(anyhow::anyhow!("IPFS not configured"));
}
let url = format!("{}/api/v0/cat?arg={}", ipfs_api.trim_end_matches('/'), cid);
let resp = reqwest::Client::new().post(&url).send().await?;
if !resp.status().is_success() {
return Err(anyhow::anyhow!("ipfs cat {cid}: {}", resp.status()));
}
Ok(resp.bytes().await?.to_vec())
}

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate cat implementation =="
ls -la crates/gitlawb-node/src || true
rg -n "pub async fn cat\\(" crates/gitlawb-node/src/ipfs_pin.rs
sed -n '60,110p' crates/gitlawb-node/src/ipfs_pin.rs

echo
echo "== Check for timeouts in this file =="
rg -n "\\.(timeout|connect_timeout|read_timeout|write_timeout)\\(" crates/gitlawb-node/src/ipfs_pin.rs || true
rg -n "tokio::time::timeout\\(" crates/gitlawb-node/src/ipfs_pin.rs || true

echo
echo "== Find call sites of cat() =="
rg -n "ipfs_pin::cat\\(|\\bcat\\(ipfs_api, cid\\)|\\bcat\\([^\\)]*ipfs_api" crates/gitlawb-node/src || true

echo
echo "== Search repo for reqwest client timeout usage =="
rg -n "reqwest::Client::new\\(|Client::builder\\(|\\.timeout\\(Duration::|tokio::time::timeout\\(" crates || true

Repository: Gitlawb/node

Length of output: 6214


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Inspect api/encrypted.rs around cat usage =="
sed -n '1,120p' crates/gitlawb-node/src/api/encrypted.rs
echo
rg -n "ipfs_pin::cat\\(|crate::ipfs_pin::cat\\(|cat\\(" crates/gitlawb-node/src/api/encrypted.rs

echo
echo "== Inspect sync.rs around cat usage =="
sed -n '250,360p' crates/gitlawb-node/src/sync.rs
echo
rg -n "ipfs_pin::cat\\(|crate::ipfs_pin::cat\\(" crates/gitlawb-node/src/sync.rs

echo
echo "== Check for tokio::time::timeout usage near these areas =="
rg -n "tokio::time::timeout\\(" crates/gitlawb-node/src/sync.rs

Repository: Gitlawb/node

Length of output: 8969


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1,120p' crates/gitlawb-node/src/ipfs_pin.rs

Repository: Gitlawb/node

Length of output: 4233


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "reqwest\s*=" -S Cargo.toml crates -g "Cargo.toml"
rg -n "name = \"reqwest\"" Cargo.lock || true
rg -n "reqwest.*version" Cargo.lock || true

Repository: Gitlawb/node

Length of output: 400


🌐 Web query:

reqwest 0.12 RequestBuilder .timeout(Duration) API

💡 Result:

In reqwest 0.12, the RequestBuilder::timeout(Duration) method is used to configure a timeout for an individual request [1][2]. When this method is called, the provided Duration sets a timeout that begins when the request starts the connection process and ends once the response body has been fully received [1][2]. This per-request timeout overrides any timeout that may have been globally configured on the Client via ClientBuilder::timeout [1][2]. The method takes a std::time::Duration as an argument and returns the modified RequestBuilder, allowing for a fluent builder pattern [1]. This API is available in both the asynchronous reqwest::RequestBuilder and the blocking reqwest::blocking::RequestBuilder [1][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "tokio::time::timeout\\(" crates/gitlawb-node/src/api/encrypted.rs crates/gitlawb-node/src/sync.rs || true
rg -n "ipfs_pin::cat\\(" crates/gitlawb-node/src/api/encrypted.rs crates/gitlawb-node/src/sync.rs

Repository: Gitlawb/node

Length of output: 282


Add a bounded timeout to IPFS cat requests

crates/gitlawb-node/src/ipfs_pin.rs’s cat does a reqwest::Client::new().post(...).send().await? without any explicit request timeout, and the ipfs_pin::cat(...) call sites aren’t wrapped in a tokio::time::timeout(...), so a stalled Kubo can hang the handler indefinitely.

Suggested patch
+use std::time::Duration;
+
 pub async fn cat(ipfs_api: &str, cid: &str) -> Result<Vec<u8>> {
@@
-    let resp = reqwest::Client::new().post(&url).send().await?;
+    let resp = reqwest::Client::new()
+        .post(&url)
+        .timeout(Duration::from_secs(15))
+        .send()
+        .await?;

Set the timeout value to match the worst-case envelope size / read latency expected in production.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/ipfs_pin.rs` around lines 75 - 86, The cat function
currently issues reqwest::Client::new().post(...).send().await? with no timeout;
update cat to use a bounded request timeout (e.g., via
reqwest::Client::builder().timeout(Duration::from_secs(<N>)).build() and then
.post(...).send().await?) or wrap the send/bytes await in
tokio::time::timeout(Duration::from_secs(<N>), ...).await and convert timeout
errors to anyhow errors; reference the cat function and ipfs_api/cid parameters
when making the change and pick a configurable constant or env-driven value for
the timeout so production worst-case envelope/read latency can be tuned.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-existing pinning code, not touched by this PR; out of scope for the recipient-set blinding change here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@beardthelion, the cat function is actually new code introduced in this PR — it appears in the diff with + lines and does not exist in the base branch. So the missing timeout does fall within the scope of this change.

That said, if you'd prefer to keep this PR focused on the recipient-set blinding and address the timeout in a follow-up, that's a reasonable call. Would you like me to open a GitHub issue to track adding a bounded timeout to ipfs_pin::cat?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I was wrong to call this pre-existing: cat() is new in this stack (introduced in the B1 fetch work, 74b7a40), so the missing timeout is in scope for the encrypted-replication change. Valid robustness issue; will address it in that lane rather than here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed as #45.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked in #45 (bounded timeout for ipfs_pin::cat). Handling it there rather than in this PR.

@kevincodex1

Copy link
Copy Markdown
Contributor

hello bro please fix conflicts!

@beardthelion beardthelion force-pushed the feat/recipient-blinding-at-rest branch from f5864fc to eab9e77 Compare June 18, 2026 16:34

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/gitlawb-node/src/git/visibility_pack.rs (1)

94-124: 💤 Low value

Consider caching blob_paths result to avoid duplicate traversal.

withheld_blob_oids (called on line 100) internally invokes blob_paths, and then line 111 calls blob_paths again. For large repositories, this duplicates the git ls-tree work across all refs.

A refactor could extract blob_paths once and pass the result to both the withholding check and the recipient computation, halving the subprocess overhead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/git/visibility_pack.rs` around lines 94 - 124, The
function withheld_blob_recipients calls blob_paths twice - once indirectly
within withheld_blob_oids and again explicitly in the for loop starting around
line 111. For large repositories, this duplicates expensive git ls-tree
operations. Refactor the code to call blob_paths once at the beginning and pass
the result to both withheld_blob_oids and the recipient computation loop, either
by modifying withheld_blob_oids to accept the blob paths as a parameter or by
restructuring the logic to compute blob_paths once and reuse it throughout the
function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/gitlawb-node/src/git/visibility_pack.rs`:
- Around line 94-124: The function withheld_blob_recipients calls blob_paths
twice - once indirectly within withheld_blob_oids and again explicitly in the
for loop starting around line 111. For large repositories, this duplicates
expensive git ls-tree operations. Refactor the code to call blob_paths once at
the beginning and pass the result to both withheld_blob_oids and the recipient
computation loop, either by modifying withheld_blob_oids to accept the blob
paths as a parameter or by restructuring the logic to compute blob_paths once
and reuse it throughout the function.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc40e51d-90bc-4958-a4f8-5d1953837585

📥 Commits

Reviewing files that changed from the base of the PR and between f5864fc and eab9e77.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • crates/gitlawb-core/Cargo.toml
  • crates/gitlawb-core/src/encrypt.rs
  • crates/gitlawb-core/src/identity.rs
  • crates/gitlawb-core/src/lib.rs
  • crates/gitlawb-node/Cargo.toml
  • crates/gitlawb-node/src/api/encrypted.rs
  • crates/gitlawb-node/src/api/mod.rs
  • crates/gitlawb-node/src/api/repos.rs
  • crates/gitlawb-node/src/arweave.rs
  • crates/gitlawb-node/src/db/mod.rs
  • crates/gitlawb-node/src/encrypted_pin.rs
  • crates/gitlawb-node/src/git/visibility_pack.rs
  • crates/gitlawb-node/src/ipfs_pin.rs
  • crates/gitlawb-node/src/main.rs
  • crates/gitlawb-node/src/pinata.rs
  • crates/gitlawb-node/src/server.rs
  • crates/gitlawb-node/src/sync.rs
  • crates/gitlawb-node/src/visibility.rs
  • crates/gl/src/clone.rs
✅ Files skipped from review due to trivial changes (1)
  • crates/gitlawb-node/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (15)
  • crates/gitlawb-core/Cargo.toml
  • crates/gitlawb-core/src/identity.rs
  • crates/gitlawb-node/src/api/mod.rs
  • crates/gitlawb-node/Cargo.toml
  • crates/gitlawb-node/src/server.rs
  • crates/gitlawb-node/src/ipfs_pin.rs
  • crates/gitlawb-node/src/pinata.rs
  • crates/gitlawb-node/src/arweave.rs
  • crates/gitlawb-node/src/visibility.rs
  • crates/gitlawb-node/src/encrypted_pin.rs
  • crates/gitlawb-core/src/lib.rs
  • crates/gitlawb-node/src/db/mod.rs
  • crates/gitlawb-node/src/api/encrypted.rs
  • crates/gitlawb-node/src/api/repos.rs
  • crates/gitlawb-node/src/sync.rs

@beardthelion beardthelion force-pushed the feat/recipient-blinding-at-rest branch from eab9e77 to d0a059b Compare June 18, 2026 17:44
@kevincodex1

Copy link
Copy Markdown
Contributor

hello bro @beardthelion please fix rebase and fix conflicts

…adability

The origin no longer stores recipient DIDs. Migration v5 replaces the
encrypted_blobs.recipients column with an opaque, node-keyed recipients_tag
used only to detect a recipient-set change for re-seal. B1 discovery and fetch
are now gated by the same repo-readability check the git read path uses, not by
per-recipient matching; decryption is gated by the envelope crypto, so a
non-recipient who can read the repo sees a blob's {oid, cid} but cannot open it.
encrypt_and_pin keys the tag from the node seed and returns {oid, cid}; the
Arweave manifest tuple drops the now-unused recipient vec.

A DB compromise no longer reveals the reader set; recovering it would require
brute-forcing candidate DID sets against the keyed tag with the node key.
…rror

Address review on the at-rest blinding change:
- The encrypted-blobs/replicate listing returned {oid, cid} with no visibility
  check, so a non-readable repo's blob index was reachable by an unauthenticated
  caller who guessed {owner}/{repo}. Gate it by the same repo-readability check
  discovery and fetch use. For the intended case (a public repo with withheld
  subtrees) the public root keeps this open to peers; only fully non-readable
  repos are withheld, which is the desired behavior.
- encrypt_and_pin treated a recipients_tag DB read error as a cache miss and
  resealed, causing avoidable IPFS writes during a partial outage; skip and retry
  on the next push instead.
- Correct the get_encrypted_blob doc comment to describe repo-readability access.
@beardthelion beardthelion force-pushed the feat/recipient-blinding-at-rest branch from d0a059b to 6b7bf15 Compare June 19, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants